Convert `pcl::int_t` to `std::int_t` #3422

kunaltyagi · 2019-10-18T06:43:29Z

Reduces another dependency on boost satisfied by C++14

taketwo

Thanks, apart from missing deprecation, LGTM.

common/include/pcl/pcl_macros.h

taketwo · 2019-10-18T14:36:20Z

There seem to be a problem with my suggestion, we get warnings of this form:

/__w/1/s/common/include/pcl/Vertices.h:19:17: warning: 'using uint32_t = uint32_t' is deprecated: use std::uint32_t instead [-Wdeprecated-declarations]
     std::vector<uint32_t> vertices;

Not sure what is the best course of action. Do we want to force people to always use std:: prefix? If so, we should also use inside the library, right?

kunaltyagi · 2019-10-18T14:57:25Z

we should also use inside the library, right

Yes. I'll fix those

kunaltyagi · 2019-10-19T14:58:16Z

Fixing by using the following script (not exactly, modified)

for type in uint8_t int8_t .... int_fast16_t; do
  for file in grep "[ <(]$type" * -nrl; do
    # had an issue with capture gp in sed
    sed -i "s/ $type/ std::$type/g" $file
    sed -i "s/<$type/[std::$type/g" $file
    sed -i "s/($type/(std::$type/g" $file
    sed -i 's/std::std::/std::/g' $file
  done
done

The change in the last 2 commit will be large. I should have used clang-tidy, but I realized too late.

Since I used sed, some changes will affect whitespace. I haven't checked the diff to see where and will rectify them later

kunaltyagi · 2019-10-20T02:50:43Z

PTAL. No more deprecation warning regarding pcl::u*int_.*_t

taketwo · 2019-10-20T08:35:13Z

Thanks, looks good! One thing on my mind though is the deprecation message. As it stands, we get

warning: ‘using uint8_t = uint8_t’ is deprecated: use std::uint8_t instead [-Wdeprecated-declarations]

which is confusing. What do you think about deviating from our usual template and stating explicitly instead of what std::uint8_t should be used? Something like:

warning: ‘using uint8_t = uint8_t’ is deprecated: use std::uint8_t instead of pcl::uint8_t [-Wdeprecated-declarations]

kunaltyagi · 2019-10-20T12:31:18Z

Sure. That's a good idea actually. Users who use using namespace pcl will see this weird message since the deprecation is in the namespace

Deprecation message

SunBlack · 2019-10-22T10:41:15Z

Just to mention: I'm getting now an compile issue since this PR:

[ 27%] Building CXX object surface/CMakeFiles/pcl_surface.dir/src/gp3.cpp.o
/media/sf_pcl/gpu/surface/src/internal.h(47): error: qualified name is not allowed

/media/sf_pcl/gpu/surface/src/internal.h(52): error: namespace "std" has no member "uint64_type"

/media/sf_pcl/gpu/surface/src/cuda/device.h(74): error: namespace "std" has no member "uint64_type"

/media/sf_pcl/gpu/surface/src/cuda/convex_hull.cu(69): error: namespace "std" has no member "uint64_type"

/media/sf_pcl/gpu/surface/src/cuda/convex_hull.cu(69): error: class "pcl::device::Static<<error-constant>>" has no member "check"

/media/sf_pcl/gpu/surface/src/cuda/convex_hull.cu(265): error: namespace "std" has no member "uint64_type"

/media/sf_pcl/gpu/surface/src/cuda/convex_hull.cu(317): error: namespace "std" has no member "uint64_type"

/media/sf_pcl/gpu/surface/src/cuda/convex_hull.cu(323): error: namespace "std" has no member "uint64_type"

/media/sf_pcl/gpu/surface/src/cuda/convex_hull.cu(337): error: namespace "std" has no member "uint64_type"

/media/sf_pcl/gpu/surface/src/cuda/convex_hull.cu(349): error: no instance of overloaded function "thrust::sort_by_key" matches the argument list
            argument types are: (thrust::device_ptr<<error-type>>, <error-type>, thrust::device_ptr<int>)

/media/sf_pcl/gpu/surface/src/cuda/convex_hull.cu(362): error: namespace "std" has no member "uint64_type"

/media/sf_pcl/gpu/surface/src/cuda/convex_hull.cu(374): error: namespace "std" has no member "uint64_type"

/media/sf_pcl/gpu/surface/src/cuda/convex_hull.cu(375): error: namespace "std" has no member "uint64_type"

/media/sf_pcl/gpu/surface/src/cuda/convex_hull.cu(591): error: namespace "std" has no member "uint64_type"

/media/sf_pcl/gpu/surface/src/cuda/convex_hull.cu(616): error: namespace "std" has no member "uint64_type"

/media/sf_pcl/gpu/surface/src/cuda/convex_hull.cu(692): error: namespace "std" has no member "uint64_type"

/media/sf_pcl/gpu/surface/src/cuda/convex_hull.cu(734): error: namespace "std" has no member "uint64_type"

/media/sf_pcl/gpu/surface/src/cuda/convex_hull.cu(735): error: namespace "std" has no member "uint64_type"

18 errors detected in the compilation of "/tmp/tmpxft_00005dbe_00000000-15_convex_hull.compute_75.cpp1.ii".
CMake Error at pcl_gpu_surface_generated_convex_hull.cu.o.RelWithDebInfo.cmake:279 (message):
  Error generating file

kunaltyagi · 2019-10-22T12:30:43Z

I see. I probably missed the gpu module compilation (along with CI) since I'm using clang not gcc. It might just be a missing header. I'll send a patch ASAP. Sorry

SunBlack · 2019-10-22T12:57:04Z

I'm using clang6 to build the PCL ;-). I have already fixed it locally, just waiting until build is fully completed, to be sure there are no further compile issues.

It seems you didn't used best search & replace pattern, as this doesn't look as valid code^^:

pcl/gpu/surface/src/convex_hull.cpp

Line 77 in 72a0c4f

pcl::gpu::Static<sizeof(pcl::device::std::uint64_type) == 8>::check();

kunaltyagi · 2019-10-22T13:19:09Z

didn't used best search & replace pattern

Yeah. I used uint64_t since my sed threw up when I tried to incorporate \b. I thought I'd catch all compilation issues but apparently not. All the errors are in gpu with uint64_type and that further led to them passing through (grep '\b::u*int[0-9]*_t[a-zA-Z]\+\b' * -nr)

I'm using clang6 to build the PCL ;-). I have already fixed it locally

Oh, thanks a lot. Really sorry. I have clang-9 and gcc-9 and cuda doesn't like that. I switched to cuda-gcc (nvcc + gcc-8) and just started on the compilation issues but got side-tracked by:

Edited out platform-specific troubles.

SunBlack · 2019-10-22T13:41:56Z

Yeah. I used uint64_t since my sed threw up when I tried to incorporate \b.

Oh you are really using sed? I'm using it on our build server, but I don't really like it. You don't get a hint, that sed e.g. doesn't know \d, so I had to use [0-9] instead. But perl on command line isn't much better (in case you have multiline regular expressions). Don't want to know how much hours I spend in last time for this both tools for an easy task...

taketwo requested changes Oct 18, 2019

View reviewed changes

common/include/pcl/pcl_macros.h Show resolved Hide resolved

kunaltyagi force-pushed the std_int branch from 0806ca1 to 812c70c Compare October 18, 2019 14:19

Using std:: instead of pcl:: for int types

c14c585

kunaltyagi force-pushed the std_int branch 2 times, most recently from 967c527 to ac56411 Compare October 19, 2019 15:19

kunaltyagi added 5 commits October 20, 2019 21:34

Deprecating type alias

187b60f

Deprecation message

Replacing boost::int_t with std::int_t

d3ff8f2

Removing boost cstdint header

98280f1

Replacing uint* with std::uint*

f8c37bd

Replacing int* with std::int*

aa8a90c

kunaltyagi force-pushed the std_int branch from ac56411 to aa8a90c Compare October 20, 2019 12:35

Fix misalignment due to 's/size_t/std::size_t/g'

3aeeb54

taketwo approved these changes Oct 21, 2019

View reviewed changes

kunaltyagi merged commit 1e3c62e into PointCloudLibrary:master Oct 21, 2019

kunaltyagi deleted the std_int branch October 21, 2019 09:49

taketwo changed the title ~~Convert pcl::int_t to std::int_t~~ Convert pcl::int_t to std::int_t Jan 18, 2020

taketwo added the modernization label Jan 18, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert `pcl::int_t` to `std::int_t` #3422

Convert `pcl::int_t` to `std::int_t` #3422

kunaltyagi commented Oct 18, 2019

taketwo left a comment

taketwo commented Oct 18, 2019

kunaltyagi commented Oct 18, 2019

kunaltyagi commented Oct 19, 2019 •

edited

Loading

kunaltyagi commented Oct 20, 2019

taketwo commented Oct 20, 2019

kunaltyagi commented Oct 20, 2019

SunBlack commented Oct 22, 2019

kunaltyagi commented Oct 22, 2019

SunBlack commented Oct 22, 2019

kunaltyagi commented Oct 22, 2019 •

edited

Loading

SunBlack commented Oct 22, 2019

Convert pcl::int_t to std::int_t #3422

Convert pcl::int_t to std::int_t #3422

Conversation

kunaltyagi commented Oct 18, 2019

taketwo left a comment

Choose a reason for hiding this comment

taketwo commented Oct 18, 2019

kunaltyagi commented Oct 18, 2019

kunaltyagi commented Oct 19, 2019 • edited Loading

kunaltyagi commented Oct 20, 2019

taketwo commented Oct 20, 2019

kunaltyagi commented Oct 20, 2019

SunBlack commented Oct 22, 2019

kunaltyagi commented Oct 22, 2019

SunBlack commented Oct 22, 2019

kunaltyagi commented Oct 22, 2019 • edited Loading

SunBlack commented Oct 22, 2019

Convert `pcl::int_t` to `std::int_t` #3422

Convert `pcl::int_t` to `std::int_t` #3422

kunaltyagi commented Oct 19, 2019 •

edited

Loading

kunaltyagi commented Oct 22, 2019 •

edited

Loading